feat: add safe lazy route compilation#97
Conversation
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e9180b2bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds opt-in lazy route-entry compilation with eager route exclusions, native resource matching, and export bridges for generated browser manifest assets. Route-chunk transforms now use the shared post-processing wrapper. Development compilation tracking distinguishes file-backed invalidations from explicit lazy activations and coordinates CSS ownership and route-manifest notifications. Tests and the default template cover the new matching, export-bridge, invalidation, and full-lazy route activation behavior. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/modify-browser-manifest.ts (1)
274-283: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider gating the bridge pass to dev only.
lazyCompilationis a dev-only config, so lazy-compilation-proxy assets never exist during a production build. As written, this runs forwebin both dev and build, reading and regex-scanning every route-entry asset just tocontinueduring builds — wasted work, and it needlessly exposes the fail-loud throw paths (Lines 133-137, 143-166) to production builds.routeChunkOptions?.isBuildis already available in this scope.♻️ Proposed gate
- if (pluginOptions.unstableLazyCompilationRouteEntries) { + if ( + pluginOptions.unstableLazyCompilationRouteEntries && + !routeChunkOptions?.isBuild + ) { appendLazyRouteEntryExportBridges({ assets, compilation, moduleExportsByRouteId, routes, sources, stats, }); }Please confirm lazy route-entry bridges are never expected during a production build (i.e.,
__lazy-compilation-proxy__shapes only appear under devlazyCompilation).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modify-browser-manifest.ts` around lines 274 - 283, Gate the appendLazyRouteEntryExportBridges call in the unstableLazyCompilationRouteEntries block to development only by also checking that routeChunkOptions?.isBuild is false. Use the existing routeChunkOptions scope and preserve the current behavior for dev builds; confirm production builds never process lazy-compilation-proxy route-entry assets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dev-runtime-controller.ts`:
- Around line 73-78: Keep the lazy-compilation marker logic in
isExplicitLazyCompilation version-specific: avoid relying unconditionally on
Symbol.for('rspack.lazyCompilationCurrent'), and gate or isolate it based on the
supported `@rspack/core` version so incompatible changes disable or bypass the
hook safely.
---
Nitpick comments:
In `@src/modify-browser-manifest.ts`:
- Around line 274-283: Gate the appendLazyRouteEntryExportBridges call in the
unstableLazyCompilationRouteEntries block to development only by also checking
that routeChunkOptions?.isBuild is false. Use the existing routeChunkOptions
scope and preserve the current behavior for dev builds; confirm production
builds never process lazy-compilation-proxy route-entry assets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3981b74-3f1a-4b05-aa03-3ef9a8c10081
📒 Files selected for processing (16)
.changeset/full-lazy-compilation-safety.mdsrc/build-output-transforms.tssrc/dev-generation.tssrc/dev-runtime-artifacts.tssrc/dev-runtime-controller.tssrc/index.tssrc/lazy-compilation.tssrc/modify-browser-manifest.tssrc/types.tstests/build-output-transforms.test.tstests/dev-generation-css.test.tstests/dev-generation.test.tstests/dev-runtime-controller.test.tstests/index.test.tstests/lazy-compilation.test.tstests/modify-browser-manifest.test.ts
Benchmark ResultsCompared PR head Reading benchmark confidenceRaw deltas are always shown. The signal label only indicates whether the observed median delta is larger than a robust run-to-run noise band; it does not erase or replace the measurement. The noise band is the larger of 2% or two combined robust standard deviations estimated from each side's relative median absolute deviation (rMAD). Fewer than three finite samples is reported as insufficient data. An inconclusive result should be rerun or investigated from the uploaded raw samples before drawing a performance conclusion.
Dev Rollup
Production Build BenchmarksRendered 7 production build benchmarks.
ci-small+ci-large Dev Fixture SummaryRendered 7 dev benchmark fixtures from the
large-355-ssr-esm Plugin Operations
synthetic-1024-ssr-esm Plugin Operations
synthetic-1024-ssr-esm-split Plugin Operations
synthetic-256-sourcemaps Plugin Operations
synthetic-256-ssr-esm Plugin Operations
synthetic-256-ssr-esm-split Plugin Operations
synthetic-48-ssr-esm Plugin Operations
Synthetic Rsbuild AppRendered 2 production build benchmarks.
Rendered 1 dev benchmark fixture from the embedded complex app.
Profile: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 136-137: Remove the temporary pkg.pr.new override for
`@rsbuild/core` from pnpm.overrides in package.json, replacing it with a stable
published release/tag or removing the override so installs and CI no longer
depend on the preview artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97679c40-53d2-41af-a134-e4c39888842a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
package.json
|
Withdrawn: the plugin-only full-lazy workaround required application-specific eager hydration routes and does not satisfy arbitrary direct-route hydration. The branch now matches main; nothing was merged. |
1 participant
Summary
Adds opt-in lazy browser route entries with explicit eager-route exclusions, a native-safe selector, and a fail-loud client-export bridge. The browser manifest, entry client, styles, and configured hydration chain stay eager; all other route entries remain lazy.
The stable-upstream workaround classifies file-backed invalidations conservatively and suppresses only transient lazy-activation route/CSS ownership changes. Unknown or real file changes remain strict. A TODO links the explicit upstream provenance and HMR protocol work that can replace the fallback.
Related links
Tests
@rspack/core@2.1.0and@rsbuild/core@2.1.0.lazy-compilation-proxybefore activation.full-reloadsocket message.